Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces computed editor DOM access with a new reactive hook and updates multiple React components/hooks to use it; changes BlockNoteEditor headless to a mutable field driven by TipTap "mount"/"unmount" events and makes Changes
Sequence DiagramsequenceDiagram
participant Component as React Component
participant Hook as useEditorDOMElement()
participant State as useEditorState()
participant Editor as BlockNoteEditor
participant TipTap as TipTap
Component->>Hook: call useEditorDOMElement(editor?)
Hook->>State: subscribe select ctx.editor?.domElement (on: "mount")
State-->>Hook: notify editor.domElement (when available/changes)
Hook-->>Component: return editorDOMElement
Component->>Component: attach/remove event listeners / build FloatingUI refs using editorDOMElement
TipTap-->>Editor: "mount"/"unmount"
Editor-->>State: emit changes (domElement/headless)
State-->>Hook: re-emit updated element
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
nperez0111
left a comment
There was a problem hiding this comment.
I'd prefer a different solution here, this is too much. Especially for an issue we can't reliably reproduce
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/react/src/components/SuggestionMenu/hooks/useSuggestionMenuKeyboardNavigation.ts (1)
9-9: Consider removing_editorfrom the signature if external consumers don't depend on it.Since the hook is exported publicly and the parameter is unused, removal would be a breaking change. If this hook isn't consumed by external packages, dropping the argument would simplify the API and avoid confusion for maintainers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/react/src/components/SuggestionMenu/GridSuggestionMenu/hooks/useGridSuggestionMenuKeyboardNavigation.ts`:
- Around line 8-15: The hook useGridSuggestionMenuKeyboardNavigation currently
accepts an explicit _editor argument but ignores it by always using
useEditorDOMElement() from context; update the implementation to pass the
explicit editor through to useEditorDOMElement (i.e., call
useEditorDOMElement(editor) or similar) and modify useEditorDOMElement to accept
an optional editorOverride parameter that falls back to useBlockNoteEditor when
not provided so keyboard listeners target the provided editor instance and
restore correct behavior for multi-editor/custom usage.
In `@packages/react/src/hooks/useEditorDomElement.ts`:
- Around line 24-33: The handler passed to editor._tiptapEditor.on("create")
uses setInitialized(true) which is idempotent and prevents re-renders after the
first create; change the effect to update state with a non-idempotent update
(e.g. flip a boolean or increment a counter) so each "create" event forces a
render. Update the useEffect in useEditorDomElement.ts around setInitialized and
the editor._tiptapEditor.on/off("create") handlers to call setInitialized(prev
=> !prev) or setInitialized(n => n + 1) instead of setInitialized(true) so
descendant consumers see a fresh editor.domElement on every create.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c1dc22a0-e78c-431f-8f52-e29b4f4e4561
📒 Files selected for processing (10)
packages/core/src/editor/BlockNoteEditor.tspackages/react/src/components/FormattingToolbar/DefaultButtons/CreateLinkButton.tsxpackages/react/src/components/LinkToolbar/LinkToolbarController.tsxpackages/react/src/components/Popovers/PositionPopover.tsxpackages/react/src/components/SuggestionMenu/GridSuggestionMenu/GridSuggestionMenuController.tsxpackages/react/src/components/SuggestionMenu/GridSuggestionMenu/hooks/useGridSuggestionMenuKeyboardNavigation.tspackages/react/src/components/SuggestionMenu/SuggestionMenuController.tsxpackages/react/src/components/SuggestionMenu/hooks/useSuggestionMenuKeyboardNavigation.tspackages/react/src/hooks/useEditorDomElement.tspackages/react/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/editor/BlockNoteEditor.ts
useEditorDOMElement hook
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react/src/hooks/useEditorState.ts (1)
138-142: Minor: hoistEVENT_TYPESout ofwatch.It's a constant and currently re-allocated on every
watch()invocation; lifting it to module scope also lets you derive theonunion fromkeyof typeof EVENT_TYPESto keep the type and runtime map in sync.♻️ Proposed refactor
+const EVENT_TYPES = { + all: ["transaction", "create", "mount", "unmount"], + mount: ["create", "mount", "unmount"], + selection: ["selectionUpdate"], + change: ["update"], +} as const; + @@ - const EVENT_TYPES = { - all: ["transaction", "create", "mount", "unmount"], - mount: ["create", "mount", "unmount"], - selection: ["selectionUpdate"], - change: ["update"], - } as const; - for (const eventType of EVENT_TYPES[on]) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/hooks/useEditorState.ts` around lines 138 - 142, Hoist the EVENT_TYPES constant out of the watch() function to module scope so it isn't re-created on every invocation; move the object currently defined inside watch to a top-level const named EVENT_TYPES and update watch to reference it. Also derive the on union/type from keyof typeof EVENT_TYPES (instead of a separate hard-coded union) so the TypeScript type stays in sync with the runtime map; update any usages of the previous on type or inline literals to use the new derived type. Ensure imports/exports and any references to EVENT_TYPES remain consistent with the new module-scope symbol.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/react/src/hooks/useEditorState.ts`:
- Around line 138-142: Hoist the EVENT_TYPES constant out of the watch()
function to module scope so it isn't re-created on every invocation; move the
object currently defined inside watch to a top-level const named EVENT_TYPES and
update watch to reference it. Also derive the on union/type from keyof typeof
EVENT_TYPES (instead of a separate hard-coded union) so the TypeScript type
stays in sync with the runtime map; update any usages of the previous on type or
inline literals to use the new derived type. Ensure imports/exports and any
references to EVENT_TYPES remain consistent with the new module-scope symbol.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5835d25a-128a-4b75-ab97-2701623fa64f
📒 Files selected for processing (3)
packages/core/src/editor/BlockNoteEditor.tspackages/react/src/hooks/useEditorDomElement.tspackages/react/src/hooks/useEditorState.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/react/src/hooks/useEditorDomElement.ts
- packages/core/src/editor/BlockNoteEditor.ts
Summary
This PR adds a
useEditorDOMElementhook which fetches the editor DOM element. It's necessary for certain cases where components rely oneditor.domElementto be defined to attach event listeners. However, when it becomes defined, there is nothing which explicitly triggers a re-render, so the hook is needed to force that re-render.Internally,
useEditorDOMElementusesuseEditorState, which has been given a"mount"event to specifically listen for when the view becomes defined/undefined.Closes #2546
Rationale
See above.
Changes
"mount"event touseEditorState."all"event inuseEditorStatealso listen for TipTap"create"/"mount"/"unmount"events.useEditorDOMElementhook.editor.domElementwith the hook where necessary.Impact
N/A
Testing
I wasn't able to reproduce the issue in #2546 locally, and have not seen other cases of this elsewhere. Therefore, no tests have been added.
Screenshots/Video
N/A
Checklist
Additional Notes
Summary by CodeRabbit